New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use the babel-plugin-polyfill-*
packages in transform-runtime
#12845
Use the babel-plugin-polyfill-*
packages in transform-runtime
#12845
Conversation
// This file isn't maintaned anymore. | ||
// Improvements should go in the babel-plugin-polyfill-corejs2 package. | ||
|
||
export default { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These two files have just been moved from src
to scripts
. GitHub shows them as new because I changed the indentation.
} | ||
if (!useRuntimeHelpers) return; | ||
|
||
file.set("helperGenerator", name => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code has only been de-indented. You can review with https://github.com/babel/babel/pull/12845/files?w=1 to disable whitespace changes.
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit ac5ffcb:
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/41848/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM overall except for comments.
|
||
var _mapInstanceProperty = require("<CWD>/packages/babel-runtime-corejs3/core-js/instance/map"); | ||
var _mapInstanceProperty = require("@babel/runtime-corejs3/core-js/instance/map.js"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should not generate package/path/polyfill.js
when absoluteRuntime
is true
, it should be an absolute path to the polyfill.
|
||
var _Promise = require("@babel/runtime-corejs3/core-js-stable/promise"); | ||
var _Map = require("@babel/runtime-corejs3/core-js-stable/map.js"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think the extension .js
should be added since we have specified package.json#exports
.
@@ -1,6 +1,6 @@ | |||
var _regeneratorRuntime = require("<CWD>/packages/babel-runtime-corejs3/regenerator"); | |||
|
|||
var _mapInstanceProperty = require("<CWD>/packages/babel-runtime-corejs3/core-js/instance/map"); | |||
var _mapInstanceProperty = require("<CWD>/packages/babel-runtime-corejs3/core-js/instance/map.js"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here the extension is ok correct, because when using absolute or relative paths npm ignores package.json#exports
(I added it for consistency with the corejs2 transform).
This is currently broken for helpers, but the fix falls in unrelated from this PR.
At least not #12106, that needs to be fixed in regenerator itself. |
This is the same as #12583, but for
@babel/runtime
🙂